-
Notifications
You must be signed in to change notification settings - Fork 378
Move jQuery and jQuery-ujs under /client and npm #88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Comment out jquery-rails gem Remove require for jQuery and jQuery-ujs from application.js Change required order of application.js client-bundle and es5-shim Add jquery-ujs to package.json and npm shrinkwrap Remove comments about jQuery from webpack.common.config.js Remove jQuery loader expose from webpack.rails.config.js Comment out jQuery import or external jQuery since it is served from /client. Move jQuery and jQuery-ujs expose to via require to rails_only.jsx entry point.
Gemfile
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete, with some some comments as to why it's not there in the GemFile.
A few needed changes. Also, it's critical that you test this with rails s -e production so that you see if the CSRF stuff is working. The CSRF parts are not running during development, AFIK.
I was getting CSRF errors in development with jQuery being exposed in and not working from within webpack.rails_only.config.js.
I did asset:precompile for production and tested rails s -e production. There were no CSRF errors in production env.
On Sep 10, 2015, at 11:47 PM, Justin Gordon notifications@github.com wrote:
A few need changes. Also, it's critical that you test this with rails s -e production so that you see if the CSRF stuff is working. The CSRF parts are not running during development, AFIK.
—
Reply to this email directly or view it on GitHub.
Move add jquery and jquery-ujs to entry point in web.common.config.js Add expose-loader for jQuery module/loaders with web.common.config.js
... webpack.common.config.js
@justin808 Final Review please after making changes from @alexfedoseev comments about where to expose and add entry for jQuery within webpack.common.config.js.
Thank you @alexfedoseev!!!
Test pass and no CSRF issues in dev or production.
Gemfile
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# It is critical to not include any of the jquery gems when following this pattern or else you might have multiple jQuery versions.
Couple small suggests. Please let me know if we should push to heroku.
@justin808 you want to push the branch to Heroku to test first before the merge?
Branch is live on Heroku. Looks good to me.
Super. Let's merge and then push master.
Move jQuery and jQuery-ujs under /client and npm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dylangrafmyre We need to update this to ^1.1.1-1
Comment out jquery-rails gem
Remove require for jQuery and jQuery-ujs from application.js
Change required order of application.js client-bundle and es5-shim
Add jquery-ujs to package.json and npm shrinkwrap
Remove comments about jQuery from webpack.common.config.js
Remove jQuery loader expose from webpack.rails.config.js
Comment out jQuery import or external jQuery since it is served from
/client.
Move jQuery and jQuery-ujs expose to via require to rails_only.jsx entry point.
Add info to README about using jQuery with Rails and Webpack
This fixes #51